Skip to content

Conversation

@jaclync
Copy link
Contributor

@jaclync jaclync commented Aug 28, 2025

For WOOMOB-1202

Description

This PR replaces the full sync catalog endpoints with paginated products/variations API requests from decision of p1756289941252509-slack-C070SJRA8DP. The previous approach used a generate → poll status → download cycle, and the new implementation uses direct paginated requests, which will be requested with parallel batching, targeting stores with catalog size < 1000 when the full sync service is implemented and integrated with POS.

Key changes:

  • Replaced generateCatalog(), checkCatalogStatus(), downloadCatalog() with loadProducts(siteID:pageNumber:) and loadProductVariations(siteID:pageNumber:)
  • Removed unused response models and helper methods

Steps to reproduce

Just CI is sufficient since the endpoints are integrated with the app yet.

Testing information

Locally, I tested with:

do {
    // Test paginated full sync
    let allProducts = try await loadAllProducts(syncRemote: syncRemote, siteID: siteID)
    let allVariations = try await loadAllProductVariations(syncRemote: syncRemote, siteID: siteID)
    print("Fetched \(allProducts.count) products and \(allVariations.count) variations for siteID \(siteID)")
}

and helper functions:

private func loadAllProducts(syncRemote: POSCatalogSyncRemote, siteID: Int64) async throws -> [POSProduct] {
    let firstPage = try await syncRemote.loadProducts(siteID: siteID, pageNumber: 1)
    print("Total products: \(firstPage.totalItems ?? 0)")    
    guard let totalItems = firstPage.totalItems, totalItems > 0 else {
        return []
    }

    // Calculate total pages needed (page size is 100 as per constants)
    let pageSize = 100
    let totalPages = (totalItems + pageSize - 1) / pageSize // Ceiling division
    print("Total product pages to fetch: \(totalPages)")
    
    // Start with products from first page
    var allProducts = firstPage.items
    
    guard totalPages > 1 else {
        return allProducts
    }
    
    // Fetch remaining pages in parallel batches
    let remainingPages = Array(2...totalPages)
    let batchSize = 5 // Process 5 pages in parallel at a time
    
    for batch in remainingPages.chunked(into: batchSize) {
        let batchResults = try await withThrowingTaskGroup(of: (Int, PagedItems<POSProduct>).self) { group in
            // Add tasks for each page in the batch
            for pageNumber in batch {
                group.addTask {
                    print("🔄 Fetching products page \(pageNumber)/\(totalPages)")
                    let result = try await syncRemote.loadProducts(siteID: siteID, pageNumber: pageNumber)
                    return (pageNumber, result)
                }
            }
            
            // Collect results
            var results: [(Int, PagedItems<POSProduct>)] = []
            for try await result in group {
                results.append(result)
            }
            
            return results
        }
        
        // Sort results by page number to maintain order
        let sortedResults = batchResults.sorted { $0.0 < $1.0 }
        
        // Add all products from this batch in order
        for (_, pageResult) in sortedResults {
            allProducts.append(contentsOf: pageResult.items)
        }
        
        print("Completed batch, total products loaded: \(allProducts.count)")
    }
    
    print("✅ Products sync complete: \(allProducts.count) products loaded")
    return allProducts
}

private func loadAllProductVariations(syncRemote: POSCatalogSyncRemote, siteID: Int64) async throws -> [POSProductVariation] {
    // Make first request to get total count
    let firstPage = try await syncRemote.loadProductVariations(siteID: siteID, pageNumber: 1)
    print("Total variations: \(firstPage.totalItems ?? 0)")
    
    guard let totalItems = firstPage.totalItems, totalItems > 0 else {
        return []
    }
    
    // Calculate total pages needed (page size is 100 as per constants)
    let pageSize = 100
    let totalPages = (totalItems + pageSize - 1) / pageSize // Ceiling division
    print("Total pages to fetch: \(totalPages)")
    
    // Start with variations from first page
    var allVariations = firstPage.items
    
    guard totalPages > 1 else {
        return allVariations
    }
    
    // Fetch remaining pages in parallel batches
    let remainingPages = Array(2...totalPages)
    let batchSize = 5 // Process 5 pages in parallel at a time
    
    for batch in remainingPages.chunked(into: batchSize) {
        let batchResults = try await withThrowingTaskGroup(of: (Int, PagedItems<POSProductVariation>).self) { group in
            // Add tasks for each page in the batch
            for pageNumber in batch {
                group.addTask {
                    print("🔄 Fetching variations page \(pageNumber)/\(totalPages)")
                    let result = try await syncRemote.loadProductVariations(siteID: siteID, pageNumber: pageNumber)
                    return (pageNumber, result)
                }
            }
            
            // Collect results
            var results: [(Int, PagedItems<POSProductVariation>)] = []
            for try await result in group {
                results.append(result)
            }
            return results
        }
        
        // Sort results by page number to maintain order
        let sortedResults = batchResults.sorted { $0.0 < $1.0 }
        
        // Add all variations from this batch in order
        for (_, pageResult) in sortedResults {
            allVariations.append(contentsOf: pageResult.items)
        }
        
        print("Completed batch, total variations loaded: \(allVariations.count)")
    }
    
    print("✅ Variations sync complete: \(allVariations.count) variations loaded")
    return allVariations
}

  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@jaclync jaclync added type: task An internally driven task. feature: POS labels Aug 28, 2025
@jaclync jaclync added this to the 23.2 milestone Aug 28, 2025
@wpmobilebot
Copy link
Collaborator

App Icon📲 You can test the changes from this Pull Request in WooCommerce iOS Prototype by scanning the QR code below to install the corresponding build.

App NameWooCommerce iOS Prototype
Build Numberpr16050-9574f82
Version23.1
Bundle IDcom.automattic.alpha.woocommerce
Commit9574f82
Installation URL5ki76okq3b898
Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

@jaclync
Copy link
Contributor Author

jaclync commented Aug 29, 2025

Marking this PR ready for review since the current consensus from p1756289941252509-slack-C070SJRA8DP is to go with the paginated approach until we know the catalog API status. I won't merge until an official decision is posted.

I decided not to include the modified_before parameter that I was considering before p1756361691914349/1756289941.252509-slack-C070SJRA8DP, as updated items during the whole sync time could mess up the paged items anyway.

@jaclync jaclync requested a review from joshheald August 29, 2025 06:21
Copy link
Contributor

@joshheald joshheald left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works well, thanks.

Are you sure you want to replace all the Catalog code though? It's likely that we'll still need it soon anyway – we didn't have much confidence that the hosted catalog will actually work for our use case, unless it's combined with something very similar to what we've talked about already.

If you think it'll be easy to recreate from source control, or wasn't far enough along to have much valuable, then maybe removal is best... but up to you 😊

@jaclync
Copy link
Contributor Author

jaclync commented Sep 1, 2025

Are you sure you want to replace all the Catalog code though? It's likely that we'll still need it soon anyway – we didn't have much confidence that the hosted catalog will actually work for our use case, unless it's combined with something very similar to what we've talked about already.

If you think it'll be easy to recreate from source control, or wasn't far enough along to have much valuable, then maybe removal is best... but up to you 😊

Given that there are no concrete technical discussions about the hosted catalog solution (that I know of, feel free to share any), I don't see a conclusion happening in the next few weeks. The code can be recovered by the diffs in #16041 and #16042 in this remote, and the WIP background download can be resumed from #16048.

--

Merging now from pdfdoF-819-p2.

@jaclync jaclync merged commit f2e3e2e into trunk Sep 1, 2025
26 of 28 checks passed
@jaclync jaclync deleted the feat/WOOMOB-1202-update-catalog-sync-remote-to-paginated-requests branch September 1, 2025 05:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature: POS type: task An internally driven task.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants